Rework PluginRegistry API and introduce VersionMatchRule enum #1163

HannesWell · 2024-02-21T23:59:12Z

Modernize the PluginRegistry API that contains the antiquated org.eclipse.osgi.service.resolver.VersionRange by providing alternatives using the org.osgi.framework.VersionRange and deprecating the existing methods for removal.

In conjunction with that also mark PluginRegistry.PluginFilter as deprecated for removal since it is effectively a Predicate<IPluginModelBase> and just use the latter in the new methods.

This is currently an early draft to discuss the overall direction. Since the intention is to focus on the API callers of the now deprecated methods are not replaced in this PR. I'll do that in a follow-up.

About the API I'm also contemplating to further modernize the methods in PluginRegistry that now return a List to return a Stream instead. Then we could also save the filter arguments since callers can just filter the returned stream if desired.

Furthermore I would also like to replace the integer and string constants in IMatchRules by a corresponding Enum VersionMatchRules (or similar) that also provides method to obtain a predicate for a reference version that implements the literals matching rule or to obtain a corresponding VersionRange from a reference version (which ever is needed).

Part of #1069

github-actions · 2024-02-22T00:10:00Z

Test Results

285 files ±0 285 suites ±0 47m 32s ⏱️ - 1m 8s
3 580 tests ±0 3 504 ✅ ±0 76 💤 ±0 0 ❌ ±0
10 932 runs ±0 10 701 ✅ ±0 231 💤 ±0 0 ❌ ±0

Results for commit cbb7f76. ± Comparison against base commit 969eb9b.

♻️ This comment has been updated with latest results.

HannesWell · 2024-07-15T22:46:06Z

This PR now focus only on the rework of the PluginRegistry API and has a adjustments of the pde.project description API moved out into #1340.

In the meantime I made greater changes to the PluginRegistry.findModel(s)() methods and introduced a VersionMatchRule enum. VersionMatchRule represents the rules currently defined in IMatchRules as int constants and handled in various places in the PDE code base in a type-safe manner and also provides the represented logic in one place.

The rework is not yet completely finished, but the overall direction is settled and I would like to gather feedback already.
On the long run the goal is also to replace IMatchRules respectively the usage of int literals by the new VersionMatchRule enum.

HannesWell · 2024-07-15T22:47:32Z

ui/org.eclipse.pde.core/src/org/eclipse/pde/core/plugin/PluginRegistry.java

 	 */
+	@Deprecated(forRemoval = true, since = "3.19 (removal in 2026-09 or later)")


Btw. what do you think about mentioning the earliest removal date here?
I think it would help consumers to understand better when the actual removal will happen earliest.

HannesWell · 2024-07-15T22:55:44Z

ui/org.eclipse.pde.core/src/org/eclipse/pde/core/plugin/PluginRegistry.java

+	public static Stream<IPluginModelBase> findModels(String id, String version, VersionMatchRule matchRule) {
+		Version reference = version != null ? Version.valueOf(version) : null;
+		return selectModels(id, version != null ? p -> matchRule.matches(VersionUtil.getVersion(p), reference) : null);
+	}


Instead of having one overload of findModel(s) that accepts a VersionRange and one that accepts a version String plus a VersionMatchRule we could also just provide one method with a VersionRange.
With the new VersionMatchRule one would then use

findModel("foo.bar", EQUIVALENT.rangeFor(version))

instead of

findModel("foo.bar", versionStr, EQUIVALENT)

Of course the level of convenience is also influenced by the fact if one has a Version object or a version String.
Maybe this justifies to keep both so in case one has a Version object the former solution can be used and if one has a version String the latter approach is nicer.

I'm torn on this question.

Because there is more than just one difference as described above, I think it is fine to preserve this structure.

HannesWell · 2024-08-06T22:46:22Z

In general I think this rework is now ready, although there might be a few more minor adjustments necessary.

So anybody interested in this change, please share your opinion as soon as possible.
Otherwise I plan to complete this by the end of this week.

I have also evaluated the possibility to return Optional<IPluginModelBase> in the new overloads of PluginRegistry.findModel() that return null if nothing was found. But this creates an odd asymmetry with respect to the existing overloads. I find it more important to keep the API consistent here. In the other overload that was added recently, we also continued to return null: #1062

Besides replacing the Equinox resolver VersionRange by the OSGi's org.osgi.framework.VersionRange, this also removes the rarely used filter parameter from the findModel() methods. Instead of an array the new findModels() methods return a stream that can be filtered by the caller if desired and is sorted by descending plug-in version. In all cases plug-ins with null IPluginBase or ID are now discarded by all findModel() methods that accept plug-in id plus version specification (range or match-rule). Part of eclipse-pde#1069

HannesWell force-pushed the version-cleanUps branch from 10be7e4 to 6e29ee0 Compare February 22, 2024 00:00

HannesWell mentioned this pull request Feb 22, 2024

Replace APIs using equinox-resolve's BundleDescription #1164

Draft

HannesWell mentioned this pull request Jul 14, 2024

Replace project APIs using antiquated Equinox resolver's VersionRange #1340

Merged

HannesWell force-pushed the version-cleanUps branch from 6e29ee0 to 35a14aa Compare July 15, 2024 22:23

HannesWell changed the title ~~Replace APIs using equinox VersionRange and PluginRegistry.PluginFilter~~ Rework PluginRegistry API and introduce VersionMatchRule enum Jul 15, 2024

HannesWell force-pushed the version-cleanUps branch from 35a14aa to fead832 Compare July 15, 2024 22:34

HannesWell commented Jul 15, 2024

View reviewed changes

HannesWell mentioned this pull request Jul 15, 2024

Modernize pde.project description interfaces #1341

Merged

HannesWell force-pushed the version-cleanUps branch from fead832 to f6d15ec Compare July 22, 2024 23:12

HannesWell force-pushed the version-cleanUps branch 4 times, most recently from 7896430 to 4551e03 Compare August 6, 2024 22:31

HannesWell marked this pull request as ready for review August 7, 2024 06:28

HannesWell force-pushed the version-cleanUps branch 4 times, most recently from 176a783 to 0f3d38f Compare August 15, 2024 21:26

HannesWell added 3 commits August 15, 2024 23:34

Introduce VersionMatchRule Enum as replacement for IMatchRules interface

46f4284

Migrate to reworked PluginRegistry.findModel() API

cbb7f76

HannesWell force-pushed the version-cleanUps branch from 0f3d38f to cbb7f76 Compare August 15, 2024 21:35

HannesWell merged commit 4a9c2a1 into eclipse-pde:master Aug 15, 2024
12 of 13 checks passed

HannesWell deleted the version-cleanUps branch August 15, 2024 22:02

HannesWell mentioned this pull request Aug 15, 2024

Migrate PDE off the antiquated Equinox resolver and its API #1069

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework PluginRegistry API and introduce VersionMatchRule enum #1163

Rework PluginRegistry API and introduce VersionMatchRule enum #1163

HannesWell commented Feb 21, 2024 •

edited

Loading

github-actions bot commented Feb 22, 2024 •

edited

Loading

HannesWell commented Jul 15, 2024

HannesWell Jul 15, 2024

HannesWell Jul 15, 2024 •

edited

Loading

HannesWell Aug 6, 2024

HannesWell commented Aug 6, 2024

		*/
		@Deprecated(forRemoval = true, since = "3.19 (removal in 2026-09 or later)")

Rework PluginRegistry API and introduce VersionMatchRule enum #1163

Rework PluginRegistry API and introduce VersionMatchRule enum #1163

Conversation

HannesWell commented Feb 21, 2024 • edited Loading

github-actions bot commented Feb 22, 2024 • edited Loading

Test Results

HannesWell commented Jul 15, 2024

HannesWell Jul 15, 2024

Choose a reason for hiding this comment

HannesWell Jul 15, 2024 • edited Loading

Choose a reason for hiding this comment

HannesWell Aug 6, 2024

Choose a reason for hiding this comment

HannesWell commented Aug 6, 2024

HannesWell commented Feb 21, 2024 •

edited

Loading

github-actions bot commented Feb 22, 2024 •

edited

Loading

HannesWell Jul 15, 2024 •

edited

Loading